[DRAFT] Status should ensure CNI Network Configuration and CNI binaries in the NetConf are present to set Network Ready #116
Conversation
MikeZappa87
force-pushed
the
bug/musthavebinaries
branch
6 times, most recently
from
a3a2a92 to
545b30e
Compare
cni.go
Outdated
| @@ -140,6 +141,19 @@ func (c *libcni) Status() error { | |||
| if len(c.networks) < c.networkCount { | |||
| return ErrCNINotInitialized | |||
| } | |||
|
|
|||
| for _, dir := range c.pluginDirs { | |||
There was a problem hiding this comment.
nit.. per discussion offline we could/should do this check on load instead.. TODO: consider if need to add a listener on the plugindirs to know when to reload as we do on the config path
There was a problem hiding this comment.
hesitation on doing to much here given the better design coming with the kni/cni daemons.. might want to ignore the TODO for now
MikeZappa87
force-pushed
the
bug/musthavebinaries
branch
2 times, most recently
from
c955814 to
5d2d968
Compare
opts.go
Outdated
| plugin := plug.Network.Type | ||
|
|
||
| if !fileExistsInDir(c.pluginConfDir, plugin) { | ||
| return ErrCNIPluginNotFound |
There was a problem hiding this comment.
| return ErrCNIPluginNotFound | |
| return fmt.Errorf("CNI plugin not found in %s/%s: %w", c.pluginConfDir, plugin, ErrInvalidConfig) |
There was a problem hiding this comment.
thoughts? Not following the need to have a plurality of different error types for what amounts to incorrect cni config..
There was a problem hiding this comment.
My thought from the reader of the error message if I see the invalid cni config, I'm going to look at /etc/cni/net.d. If I see cni plugin not found, I'm looking at /opt/cni/bin. This helps reduce the troubleshooting time. The error message could be improved to give the reader the directories. I'm just thinking about it that way. Invalid config means something much different than cni plugin not found.
MikeZappa87
force-pushed
the
bug/musthavebinaries
branch
from
b78ecc0 to
0252662
Compare
|
I need to handle the case where multiple cni bin directories are specified. The binary needs to exist in one of these directories and the last checked directory could have the binary. In the current the first would be checked and fail. |
MikeZappa87
force-pushed
the
bug/musthavebinaries
branch
from
46c7c4f to
6ec6275
Compare
Signed-off-by: Michael Zappa <[email protected]>
MikeZappa87
force-pushed
the
bug/musthavebinaries
branch
from
d8657e1 to
5d1202a
Compare
errors.go
Outdated
| ErrInvalidResult = errors.New("invalid result") | ||
| ErrLoad = errors.New("failed to load cni config") | ||
| ErrCNIPluginNotFound = errors.New("cni plugin not found") | ||
| ErrCNIPluginDirNotFound = errors.New("cni plugin directory not found") |
Signed-off-by: Michael Zappa <[email protected]>
Signed-off-by: Michael Zappa <[email protected]>
Signed-off-by: Michael Zappa <[email protected]>
Signed-off-by: Michael Zappa <[email protected]>
MikeZappa87
changed the title
Signed-off-by: Michael Zappa <[email protected]>
Signed-off-by: Michael Zappa <[email protected]>
Signed-off-by: Michael Zappa <[email protected]>
It seems like issues are popping up where the CNI network configuration is present but no CNI binaries present and the node is being marked ready.